Skip to content

Find/Replace overlay: adopt Eclipse command infrastructure for shortcuts and enablement#4192

Open
HeikoKlare wants to merge 1 commit into
eclipse-platform:masterfrom
HeikoKlare:findreplace-overlay-eclipse-commands-v2-focus-service
Open

Find/Replace overlay: adopt Eclipse command infrastructure for shortcuts and enablement#4192
HeikoKlare wants to merge 1 commit into
eclipse-platform:masterfrom
HeikoKlare:findreplace-overlay-eclipse-commands-v2-focus-service

Conversation

@HeikoKlare

@HeikoKlare HeikoKlare commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Motivation

The Find/Replace overlay currently dispatches its keyboard shortcuts through custom SWT KeyListeners and toggles button enablement by manipulating ToolItems directly. This bypasses the Eclipse key binding framework entirely:

  • Shortcuts cannot be seen, or customized, via Preferences > General > Keys — the overlay's operations don't show up there at all today.
  • Any user-defined key binding override is silently ignored, since the overlay never asks the binding framework what key is currently bound to a given operation.
  • Button enablement and shortcut hints are pushed onto widgets imperatively from several call sites, rather than being derived from a single source of truth.

This continues the incremental refactoring of the overlay's command handling started in #4164, #4169, #4176, #4178, #4184, and #4185 (see "Builds upon" below): now that a dedicated FindReplaceOverlayCommandSupport class owns all the overlay's command-related infrastructure, this change lets it take the next step and represent every overlay operation as a genuine Eclipse handler.

Contributes to:

Concept and design

Actions become handlers. FindReplaceOverlayAction now extends AbstractHandler, so every overlay operation (search forward/backward, replace, toggle options, close, ...) is a real Eclipse handler that can be registered with the command framework. AccessibleToolItem.setAction() derives a ToolItem's enabled state (via IHandlerListener) and its shortcut-hint tooltip (resolved through IBindingService) from the action itself, instead of having both pushed to it explicitly at each call site.

A dedicated key-binding context hierarchy is declared in plugin.xml and activated/deactivated together with overlay focus, so that search- and replace-specific commands are only reachable while the corresponding input field has focus:

findReplaceOverlay                      (parent, active whenever the overlay has focus)
├─ findReplaceOverlay.searchFocused     (active while the search field has focus)
└─ findReplaceOverlay.replaceFocused    (active while the replace field has focus)

Handler activation is registered once per action, scoped by an ACTIVE_FOCUS_CONTROL-based Expression — backed by IFocusService tracking the search/replace text controls — rather than being imperatively activated and deactivated on every focus-gained/-lost event. The overlay's own commands are private to it (no other part registers a competing handler for them), so registering on the workbench-level IHandlerService is sufficient to keep them reliably reachable, regardless of how the target editor's own context nesting behaves.

The shortcut-hint tooltip still depends on which context is currently active (not on handler activation, which is now permanent), so it continues to be recomputed on every focus transition, exactly as before — just through a single, generic hint-refresh step instead of being interleaved with handler (de)activation.

Out of scope for this change: suppression of the target editor's native text-widget shortcuts (Ctrl+Z/C/V/X/A, arrow keys, etc., so that they act on the overlay's input fields rather than the editor) continues to rely on the existing reflective setActionActivation/global-action-handler mechanism, unchanged. Replacing that reflection-based workaround is intentionally left to a follow-up (see below) to keep this change focused and reviewable.

Builds upon

This is the next step in a series of incremental refactorings of the Find/Replace overlay's command handling:

Follow-ups

This change is itself a stepping stone. Planned follow-ups building on the same command-infrastructure foundation:

  • Replace the reflective setActionActivation/global-action-handler suppression of the target editor's native shortcuts with a proper, non-reflective mechanism built on the same handler/expression infrastructure introduced here.
  • Decouple FindReplaceOverlay from StatusTextEditor, so the overlay can be used with other kinds of targets.

How to test

Manually:

  1. Open an editor, invoke Find/Replace (Ctrl+F) to open the overlay.
  2. Open Preferences > General > Keys and filter for "Find and Replace" — the overlay's commands (search forward/backward, replace, toggle options, close, ...) should be listed and rebindable.
  3. Rebind one of them (e.g. "Find and Replace: Search Forward") to a different key, apply, and confirm the new shortcut works while the search field has focus, and that the button's tooltip reflects the new binding.
  4. Confirm shortcut hints in the button tooltips only show up for the bar that currently has focus: focus the search field and hover over a replace-only button (and vice versa after opening the replace bar) — no stale/incorrect hint should be shown.
  5. Confirm Ctrl+F while the overlay has focus closes the overlay rather than opening a duplicate dialog.
  6. Confirm native text editing shortcuts (Ctrl+Z/C/V/X/A, arrow keys, Home/End, etc.) still operate on the overlay's search/replace fields rather than the underlying editor.

Automated: the existing FindReplaceOverlayTest suite (org.eclipse.ui.workbench.texteditor.tests) exercises the overlay's search/replace/history/option behavior and passes unchanged.

Screenshot

image

State of Work

I will still give this PR some polishment and probably extract out some further preparatory changes in addition to the ones already mentioned above before submitting this PR. I still wanted to provide it as draft for making aware of this work and for early testing/feedback already.

@github-actions

github-actions Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Test Results

   855 files  ±0     855 suites  ±0   54m 29s ⏱️ - 1m 58s
 8 120 tests ±0   7 877 ✅ ±0  243 💤 ±0  0 ❌ ±0 
20 262 runs  ±0  19 608 ✅ ±0  654 💤 ±0  0 ❌ ±0 

Results for commit a589b5e. ± Comparison against base commit 429f446.

♻️ This comment has been updated with latest results.

@HeikoKlare
HeikoKlare force-pushed the findreplace-overlay-eclipse-commands-v2-focus-service branch from 4933ee0 to 789755d Compare July 18, 2026 22:24
@HeikoKlare
HeikoKlare force-pushed the findreplace-overlay-eclipse-commands-v2-focus-service branch 3 times, most recently from 8857121 to 4c0c5ae Compare July 19, 2026 17:50
@HeikoKlare
HeikoKlare force-pushed the findreplace-overlay-eclipse-commands-v2-focus-service branch 3 times, most recently from 4879dca to ba82725 Compare July 20, 2026 20:25
@HeikoKlare
HeikoKlare force-pushed the findreplace-overlay-eclipse-commands-v2-focus-service branch 2 times, most recently from 6dddd7b to b122696 Compare July 21, 2026 15:06
@HeikoKlare
HeikoKlare requested a review from Copilot July 21, 2026 17:06

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR continues the Find/Replace overlay refactoring by moving shortcut dispatch and enablement/tooltip hint derivation onto Eclipse’s command + handler + key binding infrastructure, so overlay operations become rebindable and visible in Preferences > General > Keys.

Changes:

  • Convert overlay operations into command-backed AbstractHandler instances and activate them via IHandlerService with an overlay-focus expression.
  • Introduce dedicated overlay key-binding contexts (overlay/search-focused/replace-focused) and default bindings via plugin.xml.
  • Replace custom SWT key listener shortcut dispatch and compute tooltip shortcut hints via IBindingService.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceShortcutUtil.java Removes SWT KeyListener-based shortcut dispatch in favor of command framework.
bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlaySearchOptionAction.java Plumbs command IDs into option-toggle actions.
bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlayCommandSupport.java Adds context activation, handler activation, focus tracking, and hint refresh orchestration.
bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlayAction.java Makes actions real handlers and derives shortcut hints from IBindingService.
bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlay.java Wires overlay UI actions to command IDs; tracks focus for context/hint behavior.
bundles/org.eclipse.ui.workbench.texteditor/plugin.xml Declares commands, contexts, and default key bindings for the overlay.
bundles/org.eclipse.ui.workbench.texteditor/plugin.properties Adds user-facing command/context names for the new declarations.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@HeikoKlare
HeikoKlare force-pushed the findreplace-overlay-eclipse-commands-v2-focus-service branch from b122696 to 8688ab6 Compare July 21, 2026 18:21
The overlay previously dispatched keyboard shortcuts through custom SWT
KeyListeners and toggled button enablement by manipulating ToolItems
directly, bypassing the Eclipse key binding framework entirely:
shortcuts
could not be seen or customized via Preferences > Keys, and user-defined
overrides were silently ignored.

FindReplaceOverlayAction now extends AbstractHandler, so every overlay
operation is a real Eclipse handler. AccessibleToolItem.setAction()
derives a ToolItem's enabled state and shortcut-hint tooltip from the
action itself (via IHandlerListener and a hint resolved through
IBindingService), instead of having them pushed to it explicitly at each
call site.

A dedicated key-binding context hierarchy, declared in plugin.xml and
activated/deactivated with overlay focus, scopes the search- and
replace-specific commands separately from the ones common to the whole
overlay:

  findReplaceOverlay (parent, active while the search or replace field
has focus)
  |- findReplaceOverlay.searchFocused (active while the search field has
focus)
  |- findReplaceOverlay.replaceFocused (active while the replace field
has focus)

Handler activation for the overlay's own commands is registered once per
action, scoped by an ACTIVE_FOCUS_CONTROL-based expression backed by
IFocusService tracking the search/replace text controls, rather than
imperatively activated and deactivated on every focus-gained/-lost
event.

Context activation/deactivation is made defensively idempotent rather
than
relying on focus-gained/-lost events always alternating strictly:
activating
a bar's context first clears any contexts still active from a previous
activation, and disposing the overlay's container control also
deactivates
any active contexts and reactivates the target editor's actions, since
disposal is not guaranteed to always be preceded by a focus-lost event.

Fixes
eclipse-platform#2015
Prepares
eclipse-platform#1912
@HeikoKlare
HeikoKlare force-pushed the findreplace-overlay-eclipse-commands-v2-focus-service branch from 8688ab6 to a589b5e Compare July 21, 2026 19:03
@HeikoKlare
HeikoKlare marked this pull request as ready for review July 21, 2026 19:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants